feat(vinumc): expose args to external library #97
Conversation
artP2
force-pushed
the
external-context
branch
6 times, most recently
from
8535824 to
820bf8d
Compare
artP2
changed the title
artP2
changed the title
artP2
force-pushed
the
external-context
branch
from
820bf8d to
433e9cf
Compare
artP2
force-pushed
the
external-context
branch
from
433e9cf to
712e04b
Compare
|
|
||
| struct return_value ctx_get_text(call_ctx ctx); | ||
| struct return_value ctx_call(call_ctx ctx); | ||
| struct return_value ctx_get_index(call_ctx ctx, int index); |
There was a problem hiding this comment.
No way of knowing the total amount of arguments.
No way of accessing individual parameters by name.
int ctx_get_arg_count(call_ctx ctx);
call_ctx_or_null get_arg_by_name(call_ctx ctx);| if (ctx->status == true) { | ||
| return (struct return_value){ .ptr = VUT_VEC_POP(&ctx->args), .status = true }; | ||
| } |
| if (ctx->status == true) { | ||
| return (struct return_value){ .ptr = VUT_VEC_POP(&ctx->names), .status = true }; | ||
| } |
There was a problem hiding this comment.
Needs to check if ctx->names is not empty.
| /// Returns the last element and decrements the length of the vector by one. | ||
| #define VUT_VEC_POP(arr) ((arr)->len > 0 ? (arr)->base[--(arr)->len] : (arr)->base[0]) |
There was a problem hiding this comment.
Returns (possibly) garbage values when the vector is empty, may be better to read the last element before popping. You could create a VUT_VEC_LAST macro.
| struct v_str_vec args; | ||
| struct v_str_vec names; | ||
| struct arg_vec args_req; | ||
| struct name_vec names_req; |
There was a problem hiding this comment.
Missing doc comments for added fields
| #include <vutils/vec.h> | ||
|
|
||
| struct v_str_vec VUT_VEC_DEF(char *); | ||
| struct arg_vec VUT_VEC_DEF(int); | ||
| struct name_vec VUT_VEC_DEF(char *); |
There was a problem hiding this comment.
I don't like using vutils in the header. This adds an obligatory dependency to the developer of the external lib.
To help things, you could create a vec macro that gives you the base pointer and length to put it in the ctx.
Something like that:
int* ptr;
size_t len;
VUT_VEC_GET_BASE_ARRAY(&vec, ptr, len)| } | ||
|
|
||
| cctx.status = true; | ||
| call_return = symbol_info->as.func(&cctx); |
There was a problem hiding this comment.
We can't call the the function twice on a single vinum call.
What if the external function is not idempotent? For example, a function that adds a single entry to a database: If we call it on vinum code, two entries could be added instead of just a single one.
Is it possible to pre-compute the args before calling the external function?
There was a problem hiding this comment.
Is it possible to pre-compute the args before calling the external function?
This is the biggest change, so I'll focus on it first. Precomputing the arguments is possible, which is how I did it before, but for example, to search for a name in the namespace, we would have to provide the scope and the function to search for symbols.
The current method allows for flexibility, without having to provide the internal functions to the external library. To ensure it's idempotent, I thought about requiring the external library to check if ctx.status is true before performing the operations.
I would like to discuss this thoroughly before making any changes, since it alters the entire implementation.
In any case, suggestions are welcome.
This allow the external library to get arguments from the function call by index and to get any name available on the call scope